Skip to content

Conversation

wxiaoguang
Copy link
Contributor

@wxiaoguang wxiaoguang commented Apr 2, 2022

Follows

This PR removes two unmaintained vendor packages i18n and paginater

Changes:

  • Rewrite i18n package with a more clear fallback mechanism. Fix an unstable Tr behavior.
  • To keep minimal change, the new i18n still keeps the old name. In future, many code can be merged together with the translation package.
  • In future there can be a refactoring that allow the template call locale.Tr("the.key") directly, without the redundant $.Lang.
  • Add more tests for i18n.
  • Refactor the legacy Paginater to Paginator, test cases are kept unchanged.

Trivial enhancement (no breaking for end users):

  • Use the first locale in LANGS setting option as the default, requested by a user recently (forgot the ID, maybe in discord): i18n.DefaultLocales.SetDefaultLang(setting.Langs[0]), and there is a log to prevent from surprising users.

The UI with i18n still works as before (of course ...), screenshot:

image

@wxiaoguang wxiaoguang changed the title Refactor legacy Refactor legacy unmaintained packages: i18n/paginater Apr 2, 2022
@wxiaoguang wxiaoguang changed the title Refactor legacy unmaintained packages: i18n/paginater Refactor legacy unmaintained packages, support change default locale Apr 2, 2022
@wxiaoguang wxiaoguang added this to the 1.17.0 milestone Apr 2, 2022
@wxiaoguang wxiaoguang added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Apr 2, 2022
@wxiaoguang wxiaoguang changed the title Refactor legacy unmaintained packages, support change default locale Remove legacy unmaintained packages, refactor to support change default locale Apr 2, 2022
@codecov-commenter

This comment was marked as outdated.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 2, 2022
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Apr 2, 2022
@silverwind
Copy link
Member

silverwind commented Apr 2, 2022

One future idea regarding i18n in templates: It should be made a helper function, not something that's passed with the data, because it is a pain in gotemplate to pass that data down to sub-templates. I imagine a lot of templates could be simplified if it were a helper.

Copy link
Contributor

@Gusted Gusted left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Always great to refactor away packages(less binary size 😉)

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Apr 3, 2022
@wxiaoguang wxiaoguang merged commit d242511 into go-gitea:main Apr 3, 2022
@wxiaoguang wxiaoguang deleted the refactor-legacy branch April 3, 2022 09:46
zjjhot added a commit to zjjhot/gitea that referenced this pull request Apr 3, 2022
* giteaofficial/main:
  Remove legacy unmaintained packages, refactor to support change default locale (go-gitea#19308)
  [skip ci] Updated translations via Crowdin
  Prevent intermittent NPE in queue tests (go-gitea#19301)
  Upgrade xorm/builder from v0.3.9 to v0.3.10 (go-gitea#19296)
  An attempt to sync a non-mirror repo must give 400 (Bad Request) (go-gitea#19300)
  Remove legacy `unknwon/com` package (go-gitea#19298)
  Improve package registry docs (go-gitea#19273)
  A pull-mirror repo should be marked as such on creation (go-gitea#19295)
  Refactor legacy `unknwon/com` package, improve golangci lint (go-gitea#19284)
  Skip frontend ROOT_URL check on installation page, remove unnecessary global var (go-gitea#19291)
AbdulrhmnGhanem pushed a commit to kitspace/gitea that referenced this pull request Aug 24, 2022
…lt locale (go-gitea#19308)

Remove two unmaintained vendor packages `i18n` and `paginater`. Changes:
* Rewrite `i18n` package with a more clear fallback mechanism. Fix an unstable `Tr` behavior, add more tests.
* Refactor the legacy `Paginater` to `Paginator`, test cases are kept unchanged.

Trivial enhancement (no breaking for end users):
* Use the first locale in LANGS setting option as the default, add a log to prevent from surprising users.
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/refactoring Existing code has been cleaned up. There should be no new functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants